-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve update call in editing workbench #3168
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ddbd905
to
89bb7dd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3168 +/- ##
==========================================
- Coverage 85.47% 85.47% -0.01%
==========================================
Files 1279 1279
Lines 28161 28184 +23
Branches 7525 7530 +5
==========================================
+ Hits 24070 24089 +19
- Misses 4091 4095 +4
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
/hold |
89bb7dd
to
0d9347a
Compare
/unhold |
0d9347a
to
025c97a
Compare
025c97a
to
3af201e
Compare
JIRA: https://issues.redhat.com/browse/RHOAIENG-1157
Description
After discussions within the eng team, we decided not to go with the UX design and simply create some actions in the alert to allow the user to override the value and force update the workbench or abandon the value and refresh the page.
The PR does the following things:
mergePatchUpdateNotebook
to override the values in the workbench using merge-patchupdateNotebook
(which will usePUT
method instead ofPATCH
), if it generates a 409 conflict because the workbench is updated on the cluster, then prompt 2 actions to allow users to force update the workbench or refresh the pageHow Has This Been Tested?
Force update
andRefresh
buttons, the first one will push all your changes in Tab 1 through and override the changes you made in Tab 2, andRefresh
will refresh the whole page and fetch the latest notebook valuesTest Impact
Update some test cases
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main